Skip to content

Update debuginfo test runner to provide more useful output #113306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 9, 2023

Conversation

tgross35
Copy link
Contributor

@tgross35 tgross35 commented Jul 3, 2023

This change makes debuginfo tests more user friendly. Changes:

  • Print all lines that fail to match the patterns instead of just the first
  • Provide better error messages that also say what did match
  • Strip leading whitespace from directives so they are not skipped if indented
  • Improve documentation and improve nesting on some related items

As an example, given the following intentional fail (and a few not shown):

// from tests/debuginfo/rc_arc.rs

// cdb-command:dx rc,d
// cdb-check:rc,d             : 111 [Type: alloc::rc::Rc<i32>]
// cdb-check:    [Reference count] : 11 [Type: core::cell FAIL::Cell<usize>]
// cdb-check:    [Weak reference count] : 2 [Type: core::cell FAIL::Cell<usize>]

The current output (tested in #113313) will show:

2023-07-04T08:10:00.1939267Z ---- [debuginfo-cdb] tests\debuginfo\rc_arc.rs stdout ----
2023-07-04T08:10:00.1942182Z
2023-07-04T08:10:00.1957463Z error: line not found in debugger output:     [Reference count] : 11 [Type: core:: cell FAIL::Cell<usize>]
2023-07-04T08:10:00.1958272Z status: exit code: 0

With this change, you are able to see all failures in that check group, as well as what parts were successful. The output is now:

2023-07-04T09:45:57.2514224Z error: check directive(s) from `C:\a\rust\rust\tests\debuginfo\rc_arc.rs` not found in debugger output. errors:
2023-07-04T09:45:57.2514631Z     (rc_arc.rs:31) `    [Reference count] : 11 [Type: core::cell FAIL::Cell<usize>]`
2023-07-04T09:45:57.2514908Z     (rc_arc.rs:32) `    [Weak reference count] : 2 [Type: core::cell FAIL::Cell<usize>]`
2023-07-04T09:45:57.2515181Z     (rc_arc.rs:41) `    [Reference count] : 21 [Type: core::sync::atomic FAIL::AtomicUsize]`
2023-07-04T09:45:57.2515452Z     (rc_arc.rs:50) `dyn_rc,d         [Type: alloc::rc::Rc<dyn$<core::fmt FAIL::Debug> >]`
2023-07-04T09:45:57.2515695Z the following subset of check directive(s) was found successfully::
2023-07-04T09:45:57.2516080Z     (rc_arc.rs:30) `rc,d             : 111 [Type: alloc::rc::Rc<i32>]`
2023-07-04T09:45:57.2516312Z     (rc_arc.rs:35) `weak_rc,d        : 111 [Type: alloc::rc::Weak<i32>]`
2023-07-04T09:45:57.2516555Z     (rc_arc.rs:36) `    [Reference count] : 11 [Type: core::cell::Cell<usize>]`
2023-07-04T09:45:57.2516881Z     (rc_arc.rs:37) `    [Weak reference count] : 2 [Type: core::cell::Cell<usize>]`
...

Which makes it easier to see what did and didn't succeed without manual comparison against the source test file.

@rustbot
Copy link
Collaborator

rustbot commented Jul 3, 2023

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Jul 3, 2023
@rust-log-analyzer

This comment has been minimized.

@tgross35
Copy link
Contributor Author

tgross35 commented Jul 4, 2023

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 4, 2023
@rust-log-analyzer

This comment has been minimized.

@tgross35 tgross35 force-pushed the debuginfo-better-output branch from 3b612a4 to 35defa2 Compare July 4, 2023 01:46
@rust-log-analyzer

This comment has been minimized.

@tgross35 tgross35 force-pushed the debuginfo-better-output branch 2 times, most recently from 796dafb to a4698c2 Compare July 4, 2023 03:49
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@tgross35 tgross35 force-pushed the debuginfo-better-output branch 2 times, most recently from 533b46b to 7871ac3 Compare July 4, 2023 07:18
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@tgross35 tgross35 force-pushed the debuginfo-better-output branch 2 times, most recently from 2726349 to c485ca4 Compare July 4, 2023 08:59
@tgross35 tgross35 changed the title Experiment making debugtests print all mismatched lines Update debuginfo test runner to provide more useful output Jul 4, 2023
@rust-log-analyzer

This comment has been minimized.

@tgross35 tgross35 force-pushed the debuginfo-better-output branch from c485ca4 to 4e1dc23 Compare July 4, 2023 10:03
@tgross35
Copy link
Contributor Author

tgross35 commented Jul 4, 2023

Alright, the output looks good so I have reverted the intentional errors and CI changes. Ready for a look.

@rustbot ready

@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 4, 2023
@tgross35 tgross35 marked this pull request as ready for review July 4, 2023 10:04
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 4, 2023
@tgross35 tgross35 force-pushed the debuginfo-better-output branch from 4e1dc23 to e62dcf6 Compare July 4, 2023 10:11
This change makes debuginfo tests more user friendly. Changes:

-   Print all lines that fail to match the patterns instead of just
    the first
-   Provide better error messages that also say what did match
-   Strip leading whitespace from directives so they are not skipped if
    indented
-   Improve documentation and improve nesting on some related items

As an example, given the following debuginfo test with intentional
fails:

```rust
// from tests/debuginfo/rc_arc.rs

// cdb-command:dx rc,d
// cdb-check:rc,d             : 111 [Type: alloc::rc::Rc<i32>]
// cdb-check:    [Reference count] : 11 [Type: core::cell FAIL::Cell<usize>]
// cdb-check:    [Weak reference count] : 2 [Type: core::cell FAIL::Cell<usize>]

// ...
```

The current output (tested in rust-lang#113313) only shows the first mismatch:

```
2023-07-04T08:10:00.1939267Z ---- [debuginfo-cdb] tests\debuginfo\rc_arc.rs stdout ----
2023-07-04T08:10:00.1942182Z
2023-07-04T08:10:00.1957463Z error: line not found in debugger output:     [Reference count] : 11 [Type: core::cell FAIL::Cell<usize>]
2023-07-04T08:10:00.1958272Z status: exit code: 0
```

With this change, you are able to see all failures in that check
group, as well as what parts were successful. The output is now:

```
2023-07-04T09:45:57.2514224Z error: check directive(s) from `C:\a\rust\rust\tests\debuginfo\rc_arc.rs` not found in debugger output. errors:
2023-07-04T09:45:57.2514631Z     (rc_arc.rs:31) `    [Reference count] : 11 [Type: core::cell FAIL::Cell<usize>]`
2023-07-04T09:45:57.2514908Z     (rc_arc.rs:32) `    [Weak reference count] : 2 [Type: core::cell FAIL::Cell<usize>]`
2023-07-04T09:45:57.2515181Z     (rc_arc.rs:41) `    [Reference count] : 21 [Type: core::sync::atomic FAIL::AtomicUsize]`
2023-07-04T09:45:57.2515452Z     (rc_arc.rs:50) `dyn_rc,d         [Type: alloc::rc::Rc<dyn$<core::fmt FAIL::Debug> >]`
2023-07-04T09:45:57.2515695Z the following subset of check directive(s) was found successfully::
2023-07-04T09:45:57.2516080Z     (rc_arc.rs:30) `rc,d             : 111 [Type: alloc::rc::Rc<i32>]`
2023-07-04T09:45:57.2516312Z     (rc_arc.rs:35) `weak_rc,d        : 111 [Type: alloc::rc::Weak<i32>]`
2023-07-04T09:45:57.2516555Z     (rc_arc.rs:36) `    [Reference count] : 11 [Type: core::cell::Cell<usize>]`
2023-07-04T09:45:57.2516881Z     (rc_arc.rs:37) `    [Weak reference count] : 2 [Type: core::cell::Cell<usize>]`
...
```

Which makes it easier to see what did and didn't succeed without
manual comparison against the source test file.
@tgross35 tgross35 force-pushed the debuginfo-better-output branch from e62dcf6 to b0a18cb Compare July 4, 2023 10:12
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=iffy

@bors
Copy link
Collaborator

bors commented Jul 8, 2023

📌 Commit b0a18cb has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 8, 2023
@bors
Copy link
Collaborator

bors commented Jul 9, 2023

⌛ Testing commit b0a18cb with merge ba37a69...

@bors
Copy link
Collaborator

bors commented Jul 9, 2023

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing ba37a69 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 9, 2023
@bors bors merged commit ba37a69 into rust-lang:master Jul 9, 2023
@rustbot rustbot added this to the 1.73.0 milestone Jul 9, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (ba37a69): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
5.6% [5.6%, 5.6%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
1.8% [1.2%, 2.2%] 8
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.6% [-2.6%, -2.6%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.3% [-2.6%, 2.2%] 9

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.3% [3.3%, 3.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.3% [-2.6%, -2.0%] 2
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 658.609s -> 658.544s (-0.01%)

@rustbot rustbot added the perf-regression Performance regression. label Jul 9, 2023
@tgross35
Copy link
Contributor Author

tgross35 commented Jul 9, 2023

I don’t think this PR can have anything to do with the regression since it’s only a test change

@tgross35 tgross35 deleted the debuginfo-better-output branch July 9, 2023 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants